Skip to content

[wip] feat: add support for storage policies#697

Draft
pascalinthecloud wants to merge 3 commits into
vmware:mainfrom
pascalinthecloud:feat/#18-handling-of-storage-policies
Draft

[wip] feat: add support for storage policies#697
pascalinthecloud wants to merge 3 commits into
vmware:mainfrom
pascalinthecloud:feat/#18-handling-of-storage-policies

Conversation

@pascalinthecloud
Copy link
Copy Markdown

@pascalinthecloud pascalinthecloud commented Apr 26, 2026

Summary

Adds per-disk storage policy support to the vsphere-iso builder (and the shared DiskConfig used by vsphere-clone).

A new optional storage_policy field on each storage {} block accepts a vSphere storage policy name. At build time the plugin resolves the name to a profile UUID via the Policy-Based Management (PBM) API and attaches a VirtualMachineDefinedProfileSpec to the disk's device config spec. The VM home files (.vmx, .nvram, etc.) are also placed under the same policy via VirtualMachineConfigSpec.VmProfile, which the vSphere API requires whenever any disk carries a per-disk profile. If the field is left empty the behaviour is unchanged — the datastore's default policy applies.

The PBM client is initialised lazily on first use from the existing VIM25 session, so no new connection or credential is needed.

Example usage:

disk_controller_type = ["pvscsi"]

storage {
  disk_size             = 20480
  disk_thin_provisioned = true
  storage_policy        = "Gold Storage Policy"
}

Type

  • fix: Bug Fix
  • feat: Feature or Enhancement
  • docs: Documentation
  • refactor: Refactoring
  • chore: Build, Dependencies, Workflows, etc.
  • other: Other (Please describe.)

Breaking Changes?

  • Yes, there are breaking changes.
  • No, there are no breaking changes.

The new field is optional. Existing templates without storage_policy are unaffected.

Tests

  • Tests have been added or updated.
  • Tests have been completed.

New unit tests:

File Test What it covers
driver/disk_test.go TestAddStorageDevices_WithStoragePolicy VirtualMachineDefinedProfileSpec is attached to the correct disk's VirtualDeviceConfigSpec; absent on disks without a policy
driver/vm_test.go TestVirtualMachineDriver_CreateVM_WithStoragePolicy CreateVM succeeds end-to-end via the govmomi simulator when a disk carries a StoragePolicyID, exercising the VmProfile branch of the VM config spec
iso/step_create_test.go TestStepCreateVM_Run_WithStoragePolicy Policy name is resolved to a UUID and forwarded as StoragePolicyID on the driver disk
iso/step_create_test.go TestStepCreateVM_Run_StoragePolicyNotFound Step halts with a clear error when the policy name does not exist on vCenter

Output:

ok  github.com/vmware/packer-plugin-vsphere/builder/vsphere/driver  4.158s
ok  github.com/vmware/packer-plugin-vsphere/builder/vsphere/iso     1.771s
ok  github.com/vmware/packer-plugin-vsphere/builder/vsphere/common  1.815s
ok  github.com/vmware/packer-plugin-vsphere/builder/vsphere/clone   0.612s

The feature was also validated against a live vCenter with a real storage policy.

Documentation

  • Documentation has been added or updated.

docs-partials/builder/vsphere/common/DiskConfig-not-required.mdx was updated with the new storage_policy field. This partial is shared between vsphere-iso and vsphere-clone docs.

Issue References

Closes #18

Release Note

- Added `storage_policy` field to `storage {}` blocks in the `vsphere-iso` and `vsphere-clone` builders, enabling per-disk vSphere storage policy assignment. (#18)

Additional Information

Implementation notes:

  • The PBM client (govmomi/pbm) is already an indirect dependency via govmomi — no new module dependency is introduced.
  • Policy name resolution happens at build time (the Run step), not during Prepare, so the error message includes the vCenter-side policy name and is actionable.
  • VirtualMachineConfigSpec.VmProfile is set to the first disk's policy when any disk carries a policy. This is required by the vSphere API and mirrors the behaviour of other tools (Terraform provider, govc). If disks have mixed policies the VM home uses the first disk's policy; a dedicated VM-home policy field can be added in a follow-up.

@tenthirtyam
Copy link
Copy Markdown
Collaborator

CI is failing. Be sure to run "make generate" and push to your fork. Also, please sign your commits per the contributing guidelines. Thanks!

@tenthirtyam tenthirtyam added the stage/awaiting-reply Stage: Awaiting Reply label Apr 26, 2026
@pascalinthecloud pascalinthecloud force-pushed the feat/#18-handling-of-storage-policies branch from a5ad88b to a1eb4e3 Compare April 27, 2026 07:07
@pascalinthecloud
Copy link
Copy Markdown
Author

Hey,
I ran "make generate" and signed off my commits. :) Should be fine now.

@tenthirtyam tenthirtyam removed the stage/awaiting-reply Stage: Awaiting Reply label Apr 27, 2026
@tenthirtyam tenthirtyam modified the milestones: v2.y.z, v2.2.0 Apr 27, 2026
@tenthirtyam tenthirtyam changed the title feat(storage): add support for storage policies in disk configuration… feat(storage): add support for storage policies Apr 27, 2026
@tenthirtyam tenthirtyam changed the title feat(storage): add support for storage policies feat: add support for storage policies Apr 27, 2026
@tenthirtyam tenthirtyam force-pushed the feat/#18-handling-of-storage-policies branch from a1eb4e3 to fa65dfc Compare April 27, 2026 20:16
@tenthirtyam
Copy link
Copy Markdown
Collaborator

Hi @pascalinthecloud 👋🏻 - I'm hoping to be able to review and test e2e later this week.

Ryan

@tenthirtyam tenthirtyam self-requested a review April 28, 2026 18:28
@tenthirtyam tenthirtyam force-pushed the feat/#18-handling-of-storage-policies branch 2 times, most recently from ac915b9 to 081aa3b Compare May 7, 2026 14:43
pascalinthecloud and others added 3 commits May 14, 2026 15:39
… and VM creation

Signed-off-by: Pascal T. <pascal@toepke.dev>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Pascal T. <pascal@toepke.dev>
@tenthirtyam tenthirtyam force-pushed the feat/#18-handling-of-storage-policies branch from 081aa3b to 84241e7 Compare May 14, 2026 19:40
// Defaults to the first controller, `(0)`.
DiskControllerIndex int `mapstructure:"disk_controller_index"`
// The name of the storage policy to apply to the disk. The storage policy
// must already exist on the vCenter Server. If not specified, the default
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Updated product terminology.

Suggested change
// must already exist on the vCenter Server. If not specified, the default
// must already exist on the vCenter instance. If not specified, the default

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update and then run make generate before committing.

Copy link
Copy Markdown
Collaborator

@tenthirtyam tenthirtyam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pascalinthecloud! 👋🏻 Thanks for the contribution, the disk-level VirtualMachineDefinedProfileSpec wiring and the VmProfile fallback for the VM home look correct.

I noticed one behavioral gap during my testing that I'd like to see addressed before this moved forward for merge:

storage_policy currently affects only the device/VM profile spec, but datastore placement still falls through to the existing logic in LocationConfig.Prepare and VCenterDriver.FindDatastore. That means if you provide a storage_policy on a cluster (or a host with more than one datastore) and omit both datastore and datastore_cluster you will still hit:

host has multiple datastores; specify the datastore name

We should ensure the when you supply a policy that PBM selects a compliant datastore.

Could you extend the PR so that when storage_policy is set and neither datastore nor datastore_cluster is specified, that PBM will resolve a compliant placement and uses the result as the effective datastore?

In the meantime, I'm going to make as draft and [wip].

@tenthirtyam tenthirtyam marked this pull request as draft May 14, 2026 21:19
@tenthirtyam tenthirtyam changed the title feat: add support for storage policies [wip] feat: add support for storage policies May 14, 2026
@tenthirtyam tenthirtyam removed the needs-review Needs Review label May 15, 2026
@tenthirtyam tenthirtyam added stage/awaiting-reply Stage: Awaiting Reply do-not-merge Do Not Merge labels May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge Do Not Merge documentation Documentation size/l Relative Sizing: Large stage/awaiting-reply Stage: Awaiting Reply

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vsphere-iso: Add support to set a VM storage policy while configuring hard disks

2 participants